Skip to content

feat: add private preview workspace foundation - #55

Merged
Pigbibi merged 1 commit into
mainfrom
codex/qar-preview-private-workspace-foundation
Jul 15, 2026
Merged

feat: add private preview workspace foundation#55
Pigbibi merged 1 commit into
mainfrom
codex/qar-preview-private-workspace-foundation

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Scope

Fresh clean-slate filesystem foundation after frozen PR #54. PR #54 remains closed evidence and is not reused.

This PR adds build_preview_workspace(report, trusted_parent): the caller supplies only a trusted stable parent; the builder creates an unpredictable mode-0700 private workspace, writes and fsyncs the exact three preview members, performs safe lstat/fstat/readback validation, then returns the path only on success. Failure cleans only the workspace created by this call.

Deliberate boundaries

  • no fixed destination, directory publish, rename, overwrite, upload/download, workflow, Pages, publisher, weekly/monthly, legacy migration, identity/store, or production integration
  • legacy preview_bundle.py behavior is unchanged; D3 must consume the new API in a later PR
  • threat model is explicitly trusted stable parent + no caller read/upload before successful return; no ctypes/openat/renameat2 hostile-ancestor portability layer

Verification

  • focused tests/test_preview_workspace.py: 6 passed
  • full python3 -m pytest -q: 287 passed
  • python3 -m compileall -q src tests: passed
  • git diff --check: passed
  • ruff unavailable locally
  • no workflow changes; actionlint not run

Design: /Users/lisiyi/Documents/Codex/2026-07-14/ba-2/outputs/qar_preview_private_workspace_foundation_design_v1_2026-07-15.md

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

Copy link
Copy Markdown

🤖 Codex PR Review

Merge allowed: 1 finding(s) reported but none are blocking

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in src/quant_advisor_research/preview_workspace.py

After mkdtemp() creates the private workspace, the code never fsyncs the trusted parent directory. On POSIX filesystems, syncing only the workspace directory and its files does not make the new parent-directory entry durable, so a crash after build_preview_workspace() returns can lose the entire workspace path even though all three members were individually fsynced. (line 211)

Suggestion: Immediately after creating the workspace (and before returning success), open and fsync trusted_parent as well as the workspace directory so the directory entry for the new workspace is crash-durable.


Review by Codex PR Review bot • PR

@Pigbibi
Pigbibi merged commit 4ea245e into main Jul 15, 2026
4 checks passed
@Pigbibi
Pigbibi deleted the codex/qar-preview-private-workspace-foundation branch July 15, 2026 01:26

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23b0d34fd1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


def test_equivalent_reports_produce_identical_bytes_in_distinct_workspaces(tmp_path):
first = build_preview_workspace(report(), tmp_path)
second = build_preview_workspace(report(), tmp_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Freeze report timestamps before comparing workspace bytes

When the two report() calls in this test straddle a one-second boundary, build_advisory_report() writes a fresh generated_at value via utc_now_iso(), so report.json, report.html, and manifest.json differ even though the workspace builder is deterministic. Reuse a single report snapshot or inject a fixed generated_at before asserting byte-for-byte equality to avoid nondeterministic CI failures.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant